Swap to using external repo for AudioReactive #331
Conversation
📝 WalkthroughWalkthroughAdds a pre-build script that patches the wled-audioreactive Changes
Sequence Diagram(s)sequenceDiagram
participant P as PlatformIO
participant S as patch_audioreactive.py
participant FS as Filesystem
participant L as wled-audioreactive library
participant B as Build
P->>S: run pre-build script
S->>FS: locate `PROJECT_LIBDEPS_DIR/$PIOENV/wled-audioreactive/library.json`
alt file exists & readable
S->>L: read `library.json`
S->>S: ensure `build.srcFilter` exists and set to ["-<*>"]
S->>L: write updated `library.json`
S-->>P: print success
else missing or error
S-->>P: print warning / no-op
end
P->>B: continue build (uses updated deps/includes)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pio-scripts/patch_audioreactive.py`:
- Around line 21-31: The code currently only sets
library_config["build"]["srcFilter"] when the key is missing; instead, check the
existing value and enforce ["-<*>"] whenever it differs: ensure library_config
has a "build" dict (create if missing), read current =
library_config["build"].get("srcFilter"), and if current != ["-<*>"] then set
library_config["build"]["srcFilter"] = ["-<*>"], write back to library_json_path
with json.dump, and print the success message; reference the symbols
library_config, "build", "srcFilter", and library_json_path to locate the
change.
| # Add srcFilter to exclude all source files from library compilation | ||
| if "build" not in library_config: | ||
| library_config["build"] = {} | ||
|
|
||
| if "srcFilter" not in library_config["build"]: | ||
| library_config["build"]["srcFilter"] = ["-<*>"] | ||
|
|
||
| with open(library_json_path, 'w') as f: | ||
| json.dump(library_config, f, indent=2) | ||
|
|
||
| print("✓ Patched wled-audioreactive library.json to exclude .cpp from compilation") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n pio-scripts/patch_audioreactive.py | head -40Repository: MoonModules/WLED-MM
Length of output: 1866
Enforce srcFilter even when it already exists
The patch only applies when srcFilter is missing. If the upstream library is updated with a different srcFilter value that still includes sources, the patch won't override it and builds will unintentionally compile those .cpp files. Update the logic to compare the current value with the desired one and enforce ["-<*>"] whenever it differs.
🛠️ Suggested fix
- # Add srcFilter to exclude all source files from library compilation
- if "build" not in library_config:
- library_config["build"] = {}
-
- if "srcFilter" not in library_config["build"]:
- library_config["build"]["srcFilter"] = ["-<*>"]
-
- with open(library_json_path, 'w') as f:
- json.dump(library_config, f, indent=2)
-
- print("✓ Patched wled-audioreactive library.json to exclude .cpp from compilation")
+ # Add srcFilter to exclude all source files from library compilation
+ build_cfg = library_config.get("build")
+ if not isinstance(build_cfg, dict):
+ build_cfg = {}
+ library_config["build"] = build_cfg
+
+ desired_filter = ["-<*>"]
+ if build_cfg.get("srcFilter") != desired_filter:
+ build_cfg["srcFilter"] = desired_filter
+
+ with open(library_json_path, 'w') as f:
+ json.dump(library_config, f, indent=2)
+
+ print("✓ Patched wled-audioreactive library.json to exclude .cpp from compilation")🤖 Prompt for AI Agents
In `@pio-scripts/patch_audioreactive.py` around lines 21 - 31, The code currently
only sets library_config["build"]["srcFilter"] when the key is missing; instead,
check the existing value and enforce ["-<*>"] whenever it differs: ensure
library_config has a "build" dict (create if missing), read current =
library_config["build"].get("srcFilter"), and if current != ["-<*>"] then set
library_config["build"]["srcFilter"] = ["-<*>"], write back to library_json_path
with json.dump, and print the success message; reference the symbols
library_config, "build", "srcFilter", and library_json_path to locate the
change.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platformio.ini (1)
197-197:⚠️ Potential issue | 🟠 MajorFix invalid PlatformIO dependency specification syntax.
The specification
https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2violates PlatformIO format. You cannot mix#(VCS reference) and@(Registry version) together. PlatformIO uses@only for registry packages and#only for git references—never both.For a Git URL, use one of:
https://github.com/softhack007/arduinoFFT.git#develop(pin to develop branch)https://github.com/softhack007/arduinoFFT.git#v1.9.2(pin to version tag if it exists)
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Line 231: The pre-build script pre:pio-scripts/patch_audioreactive.py
currently swallows failures and merely prints a warning; change it to verify the
patch succeeded and fail the build on error by either (a) validating that
library.json contains the expected modifications after applying the patch (e.g.,
checking for the expected key/value or version) and exiting non-zero when
validation fails, or (b) rethrowing the caught exception / calling sys.exit(1)
from the main entry point instead of only logging a warning so the PlatformIO
build is aborted; update the script’s exception handlers to perform this
verification and ensure any file-not-found, JSON parse, or write errors become
fatal.
| pre:pio-scripts/build_ui.py | ||
| pre:pio-scripts/conditional_usb_mode.py | ||
| pre:pio-scripts/set_repo.py | ||
| pre:pio-scripts/patch_audioreactive.py |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the patch script and verify its existence
find . -name "patch_audioreactive.py" -o -name "*patch*" -type f | head -20Repository: MoonModules/WLED-MM
Length of output: 99
🏁 Script executed:
# Check the pio-scripts directory structure
ls -la pio-scripts/ 2>/dev/null || echo "pio-scripts directory check"
git ls-files | grep -E "pio-scripts|patch" | head -20Repository: MoonModules/WLED-MM
Length of output: 1426
🏁 Script executed:
# Examine the platformio.ini file around line 231 to see the context
sed -n '225,235p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 524
🏁 Script executed:
cat pio-scripts/patch_audioreactive.pyRepository: MoonModules/WLED-MM
Length of output: 1607
Script exists and has error handling, but failures are non-critical.
The patch_audioreactive.py script is properly registered and implemented with exception handling. However, the error handling is permissive—if the patch fails, the script only prints a warning and allows the build to continue. This means if the library.json patch is not applied due to an error (file not found, JSON parse failure, etc.), the build will proceed unpatched, potentially causing compilation errors downstream that are harder to trace back to the patch failure.
Consider adding explicit verification that the patch was successfully applied, or make patch failures fatal to the build if the patched library is required for compilation.
🤖 Prompt for AI Agents
In `@platformio.ini` at line 231, The pre-build script
pre:pio-scripts/patch_audioreactive.py currently swallows failures and merely
prints a warning; change it to verify the patch succeeded and fail the build on
error by either (a) validating that library.json contains the expected
modifications after applying the patch (e.g., checking for the expected
key/value or version) and exiting non-zero when validation fails, or (b)
rethrowing the caught exception / calling sys.exit(1) from the main entry point
instead of only logging a warning so the PlatformIO build is aborted; update the
script’s exception handlers to perform this verification and ensure any
file-not-found, JSON parse, or write errors become fatal.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 1189-1198: The AR_lib_deps entry for arduinoFFT is an invalid
mixed-format spec; update the AR_lib_deps line referencing arduinoFFT (currently
"https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2") to a valid
VCS-only spec by removing the "@ 1.9.2" suffix (or alternatively pin a specific
git tag/commit after the "#" instead of using "@version"); ensure AR_lib_deps
contains a clean VCS URL for arduinoFFT so PlatformIO can resolve it.
- Around line 1612-1614: The esp8266 AR-enabled env is pulling
${common_mm.AR_lib_deps} but missing the matching build flags; update the
esp8266_2MB_S environment so its build_flags include ${common_mm.AR_build_flags}
(in the same place where build_flags = ${common.build_flags_esp8266} is
declared) so the -D USERMOD_AUDIOREACTIVE define and include paths from
common_mm.AR_build_flags are applied; ensure the change mirrors how
esp8266pro_16MB_S and ESP32 AR environments include both
${common_mm.AR_lib_deps} and ${common_mm.AR_build_flags}.
| AR_build_flags = | ||
| -D USERMOD_AUDIOREACTIVE | ||
| -D UM_AUDIOREACTIVE_USE_NEW_FFT | ||
| -I wled00 ;; Allow external AudioReactive library to access wled.h | ||
| -I $PROJECT_LIBDEPS_DIR/$PIOENV/wled-audioreactive ;; Allow main project to find audio_reactive.h | ||
| ;; WLEDMM audioreactive usermod, licensed under EUPL-1.2 | ||
| ;; NOTE: External repo needs library.json updated with "srcFilter": ["-<*>"] to prevent .cpp compilation | ||
| AR_lib_deps = | ||
| https://github.com/MoonModules/WLED-AudioReactive-Usermod.git#39fee0aebc4f7581f23723acc710ea9cacc199d1 ;; MM version 14 | ||
| https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2 ;; used for USERMOD_AUDIOREACTIVE - optimized version, 10% faster on -S2/-C3 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PlatformIO library dependency specification syntax VCS git branch version
💡 Result:
In platformio.ini, lib_deps entries use Package Specifications. There are two common cases:
1) PlatformIO Registry libraries (SemVer / version requirements)
Syntax:
<owner>/<name>@<version requirement>
Examples:
bblanchon/ArduinoJson@^6.16.1bblanchon/ArduinoJson@~5.6,!=5.4(docs.platformio.org)
2) VCS (Git) dependencies (pin branch/tag/commit with #)
Syntax:
[<name>=]<git-url>[#<commit-ish|branch|tag>]
Examples:
https://github.com/gioblu/PJON.git#v2.0(tag) (docs.platformio.org)https://github.com/user/library.git#master(branch) (docs.platformio.org)https://github.com/user/library.git#7846d8ad52f...(commit) (docs.platformio.org)MyLib=https://github.com/user/library.git#feature-branch(force local package folder name) (docs.platformio.org)
Key distinction: @ vs #
- Use
@...for Registry version constraints. - Use
#...for Git branch/tag/commit when the dependency is a VCS URL. (docs.platformio.org)
Citations:
- 1: https://docs.platformio.org/en/latest/projectconf/sections/env/options/library/lib_deps.html?utm_source=openai
- 2: https://docs.platformio.org/en/latest/projectconf/sections/env/options/library/lib_deps.html?utm_source=openai
- 3: https://docs.platformio.org/en/latest/core/userguide/lib/cmd_install.html?utm_source=openai
- 4: https://docs.platformio.org/en/latest/core/userguide/lib/cmd_install.html?utm_source=openai
- 5: https://docs.platformio.org/en/stable/core/userguide/pkg/cmd_install.html?utm_source=openai
- 6: https://docs.platformio.org/en/stable/core/userguide/pkg/cmd_install.html?utm_source=openai
🌐 Web query:
PlatformIO lib_deps format mixed version specifier git reference
💡 Result:
In lib_deps, you can’t combine the registry SemVer specifier syntax (owner/name @ ^1.2.3, ~, !=, etc.) with a VCS (git) reference (...git#branch|tag|commit) in the same dependency entry—they are two different kinds of package specifications:
- PlatformIO Registry library + SemVer constraints uses
@:bblanchon/ArduinoJson @ ^6.19.4(docs.platformio.org)
- Git (or other VCS) dependency uses a URL (or
user/repo) plus#for branch/tag/commit:https://github.com/username/HelloWorld.git#v2.13(docs.platformio.org)- You can also pin to an exact commit SHA via
#<sha1>. (community.platformio.org)
If you want “versioning” with git
You must do it by pinning to a tag (e.g., #v1.2.3) or a commit SHA—there’s no SemVer range resolution for arbitrary git URLs in lib_deps like #^1.2.3. (docs.platformio.org)
Citations:
- 1: https://docs.platformio.org/en/latest/librarymanager/dependencies.html?utm_source=openai
- 2: https://docs.platformio.org/en/latest/librarymanager/dependencies.html?utm_source=openai
- 3: https://community.platformio.org/t/versions-in-lib-deps/2699?utm_source=openai
- 4: https://docs.platformio.org/en/latest/projectconf/sections/env/options/library/lib_deps.html?utm_source=openai
Fix invalid PlatformIO lib spec for arduinoFFT.
https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2 mixes VCS syntax (#branch) with registry versioning (@version). PlatformIO does not support combining both formats in a single dependency entry. Remove the @ 1.9.2 portion to use the valid VCS-only specification.
🛠️ Suggested fix
AR_lib_deps =
https://github.com/MoonModules/WLED-AudioReactive-Usermod.git#39fee0aebc4f7581f23723acc710ea9cacc199d1 ;; MM version 14
- https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2 ;; used for USERMOD_AUDIOREACTIVE - optimized version, 10% faster on -S2/-C3
+ https://github.com/softhack007/arduinoFFT.git#develop ;; used for USERMOD_AUDIOREACTIVE - optimized version, 10% faster on -S2/-C3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AR_build_flags = | |
| -D USERMOD_AUDIOREACTIVE | |
| -D UM_AUDIOREACTIVE_USE_NEW_FFT | |
| -I wled00 ;; Allow external AudioReactive library to access wled.h | |
| -I $PROJECT_LIBDEPS_DIR/$PIOENV/wled-audioreactive ;; Allow main project to find audio_reactive.h | |
| ;; WLEDMM audioreactive usermod, licensed under EUPL-1.2 | |
| ;; NOTE: External repo needs library.json updated with "srcFilter": ["-<*>"] to prevent .cpp compilation | |
| AR_lib_deps = | |
| https://github.com/MoonModules/WLED-AudioReactive-Usermod.git#39fee0aebc4f7581f23723acc710ea9cacc199d1 ;; MM version 14 | |
| https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2 ;; used for USERMOD_AUDIOREACTIVE - optimized version, 10% faster on -S2/-C3 | |
| AR_build_flags = | |
| -D USERMOD_AUDIOREACTIVE | |
| -D UM_AUDIOREACTIVE_USE_NEW_FFT | |
| -I wled00 ;; Allow external AudioReactive library to access wled.h | |
| -I $PROJECT_LIBDEPS_DIR/$PIOENV/wled-audioreactive ;; Allow main project to find audio_reactive.h | |
| ;; WLEDMM audioreactive usermod, licensed under EUPL-1.2 | |
| ;; NOTE: External repo needs library.json updated with "srcFilter": ["-<*>"] to prevent .cpp compilation | |
| AR_lib_deps = | |
| https://github.com/MoonModules/WLED-AudioReactive-Usermod.git#39fee0aebc4f7581f23723acc710ea9cacc199d1 ;; MM version 14 | |
| https://github.com/softhack007/arduinoFFT.git#develop ;; used for USERMOD_AUDIOREACTIVE - optimized version, 10% faster on -S2/-C3 |
🤖 Prompt for AI Agents
In `@platformio.ini` around lines 1189 - 1198, The AR_lib_deps entry for
arduinoFFT is an invalid mixed-format spec; update the AR_lib_deps line
referencing arduinoFFT (currently
"https://github.com/softhack007/arduinoFFT.git#develop @ 1.9.2") to a valid
VCS-only spec by removing the "@ 1.9.2" suffix (or alternatively pin a specific
git tag/commit after the "#" instead of using "@version"); ensure AR_lib_deps
contains a clean VCS URL for arduinoFFT so PlatformIO can resolve it.
| lib_deps = ${esp8266.lib_deps} | ||
| ${common_mm.AR_lib_deps} | ||
| build_flags = ${common.build_flags_esp8266} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the esp8266_2MB_S environment section
rg -n "esp8266_2MB_S" platformio.ini -A 10 -B 2Repository: MoonModules/WLED-MM
Length of output: 1430
🏁 Script executed:
# Find where common_mm.AR_lib_deps and common_mm.AR_build_flags are defined
rg -n "AR_lib_deps|AR_build_flags" platformio.ini -B 2 -A 2Repository: MoonModules/WLED-MM
Length of output: 5568
🏁 Script executed:
# Check the structure of the file to understand the context better
wc -l platformio.iniRepository: MoonModules/WLED-MM
Length of output: 82
🏁 Script executed:
# Check for comments around the 2MB, 4MB, and 16MB environments
sed -n '1608,1625p' platformio.ini
sed -n '1638,1650p' platformio.ini
sed -n '1736,1745p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 1839
🏁 Script executed:
# Verify that common_mm.AR_build_flags is properly defined
sed -n '1189,1200p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 872
Add AR_build_flags to esp8266_2MB_S build configuration.
The environment pulls ${common_mm.AR_lib_deps} but omits ${common_mm.AR_build_flags}, which includes the -D USERMOD_AUDIOREACTIVE define and required include paths. Without these flags, the AudioReactive library will fail to compile. This is inconsistent with esp8266pro_16MB_S and all ESP32 AR-enabled environments, which include both.
✅ Suggested fix:
build_flags = ${common.build_flags_esp8266}
+ ${common_mm.AR_build_flags}
-D WLED_RELEASE_NAME=esp8266_2MB_S🤖 Prompt for AI Agents
In `@platformio.ini` around lines 1612 - 1614, The esp8266 AR-enabled env is
pulling ${common_mm.AR_lib_deps} but missing the matching build flags; update
the esp8266_2MB_S environment so its build_flags include
${common_mm.AR_build_flags} (in the same place where build_flags =
${common.build_flags_esp8266} is declared) so the -D USERMOD_AUDIOREACTIVE
define and include paths from common_mm.AR_build_flags are applied; ensure the
change mirrors how esp8266pro_16MB_S and ESP32 AR environments include both
${common_mm.AR_lib_deps} and ${common_mm.AR_build_flags}.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platformio.ini (1)
1636-1642:⚠️ Potential issue | 🟡 MinorAdd AR_build_flags to esp8266_4MB_S build configuration.
The environment pulls
${common_mm.AR_lib_deps}(lines 1640-1641) but omits${common_mm.AR_build_flags}from build_flags (line 1642). This mirrors the issue inesp8266_2MB_Sand contrasts withesp8266pro_16MB_S(line 1741), which correctly includes both. Without these flags, the AudioReactive library will fail to compile due to missing defines and include paths.✅ Suggested fix:
build_flags = ${common.build_flags_esp8266} + ${common_mm.AR_build_flags} -D WLED_RELEASE_NAME=esp8266_4MB_S
| ;; WLEDMM audioreactive usermod, licensed under EUPL-1.2 | ||
| ;; NOTE: External repo needs library.json updated with "srcFilter": ["-<*>"] to prevent .cpp compilation | ||
| AR_lib_deps = | ||
| https://github.com/netmindz/WLED-AudioReactive-Usermod.git#compat-MM ;; MM version 14 |
There was a problem hiding this comment.
This should be swapped to MoonModules once we are good to merge the compat-MM PR
|
I've just done a basic test and usermod can be enabled and the settings all look as I would expect |
1 participant
Summary by CodeRabbit
Refactor
Documentation
Bug Fix / Internal
✏️ Tip: You can customize this high-level summary in your review settings.